Feature/add arrow strip marker #972
Conversation
|
Hi @ahcorde The ROS1 version of this PR used a CI mechanism to pull in and compile against unmerged branches from other repositories, which will be required to resolve the dependencies between this one and the common msgs PR adding the new ARROW_STRIP Marker type. Does such a mechanism exist in rviz2? Also, that PR featured a test Python script demonstrating rendering of the new marker type. Is there a place for these kinds of scripts in rviz2, or should writing the unit tests for the Also, if you could let me know what tools I should be using to lint & format the code, that'd be super helpful. |
|
@ahcorde |
|
Hi @clalancette, would you be able to answer the questions above? |
|
@ahcorde @clalancette Bump as I'd really like to get this merged soon |
|
This PR is block with this other PR ros2/common_interfaces#218 |
|
First, I'm sorry for the long delay. May is an extremely busy month leading up to the release, so I just didn't have time to look at this back then. I can answer some questions now.
Yes, though it is somewhat more manual. In this case, we use jobs launched on https://ci.ros2.org to do the same thing. Assuming we go forward with this and ros2/common_interfaces#218 , that's the mechanism we'll use.
Um, that's a good question. I don't really know. What I think we should probably do for now is just to keep that separate, but if you could put a link to it somewhere for testing that would be helpful.
If you follow the installation procedure on http://docs.ros.org/en/rolling/Installation/Alternatives/Ubuntu-Development-Setup.html, you should have all the tools you need. Then you can checkout this branch and the one in common_interfaces, and do: That should tell you what you need to change. |
|
do you have plan to continue working on this PR @rr-tom-noble ? |
|
Apologies for letting this get stale. I've had my head in ros1 for a while now, but we're migrating over to humble, so it's giving me more time to get into ros2 again. I'd definitely be interested in picking this back up over the coming weeks |
There was a problem hiding this comment.
tests are failing https://build.ros2.org/job/Rpr__rviz__ubuntu_noble_amd64/165/
ROS2 port of this PR
Ogre::ColourValueandmsg::ColorRGBAtypesArrowclass:set()intosetHeadLength(),setHeadDiameter(),setShaftLength(),setShaftDiameter()for clarity.setLength()method for setting the total arrow length, preserving shaft and head proportions.setShaftHeadRadio()method for setting shaft and head proportions, preserving the total arrow length.Arrowto use the new methodsArrowStripMarkerclass for rendering batches of arrows efficientlyNote:
While working on point 3, I noticed that there are inconsistencies in the way values are being passed to the (now removed)
Arrow::setmethod. The documentation suggests it accepts shaft and head diameters, whereas the usage suggests some callers are passing in radii.I've stuck to using diameter for now to preserve existing behaviour and for consistency with the original documentation. It's also more obvious now through the method names i.e.
set..Diameter(...radius)where these inconsistencies exist, so it should be clearer for a future PR.TODO: